Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-41147 Document that redirect_stdout provides the new stream as context var #21199

Conversation

PeterJCLaw
Copy link
Contributor

@PeterJCLaw PeterJCLaw commented Jun 28, 2020

In contextlib, _RedirectStream (the class behind redirect_stdout and redirect_stderr) returns the current stream target as its context variable, which allows code like this:

with redirect_stdout(io.StringIO()) as buffer:
    do_stuff()

use(buffer.getvalue())

where you capture the redirected stream without a separate line to declare the variable.

This PR intends to document this functionality.

https://bugs.python.org/issue41147

See also python/typeshed#4283

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@PeterJCLaw

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Jun 28, 2020
Doc/library/contextlib.rst Outdated Show resolved Hide resolved
"context variable" is not a clear term in light of the `contextvars`
module.
@PeterJCLaw
Copy link
Contributor Author

@JelleZijlstra would you have time to have another look at this?

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good apart from this typo. I don't have the permissions to merge this PR though.

Doc/library/contextlib.rst Outdated Show resolved Hide resolved
@csabella
Copy link
Contributor

@PeterJCLaw, please sign the CLA. Thank you!

@PeterJCLaw
Copy link
Contributor Author

@PeterJCLaw, please sign the CLA. Thank you!

Most odd. I signed it back in June when I opened this PR (I have a confirmation email to that effect), though the CLA now shows that I've not signed one. Anyway, I've signed another. The only difference I can see is that the first one I did used the email address I have on bugs.python.org; this time I've used the same email address as I commit with.

@PeterJCLaw
Copy link
Contributor Author

PeterJCLaw commented Oct 10, 2020

@csabella is there anything I can do to help progress this?

@@ -236,10 +236,11 @@ Functions and classes provided:

For example, the output of :func:`help` normally is sent to *sys.stdout*.
You can capture that output in a string by redirecting the output to an
:class:`io.StringIO` object::
:class:`io.StringIO` object. For convenience, the new stream is returned
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not necessarily a new stream.

Copy link
Contributor Author

@PeterJCLaw PeterJCLaw Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, I was meaning new in the sense of now being the one used for stdout. Perhaps "replacement" would be clearer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I missed your reaction earlier in the year. I've now made this change (in 14d7692).

Doc/library/contextlib.rst Outdated Show resolved Hide resolved
@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@PeterJCLaw
Copy link
Contributor Author

@bedevere-bot I have made the requested changes; please review again.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@iritkatriel: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from iritkatriel May 23, 2021 12:54
@iritkatriel
Copy link
Member

@rhettinger Are you happy for this to be a documented feature?

@iritkatriel
Copy link
Member

This is actually covered by a unit test: test_enter_result_is_target

def test_enter_result_is_target(self):

@iritkatriel iritkatriel added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels May 26, 2021
@iritkatriel iritkatriel merged commit 46db39d into python:main May 26, 2021
@miss-islington
Copy link
Contributor

Thanks @PeterJCLaw for the PR, and @iritkatriel for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.9.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 26, 2021
… as context var (pythonGH-21199)

(cherry picked from commit 46db39d)

Co-authored-by: Peter Law <[email protected]>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label May 26, 2021
@bedevere-bot
Copy link

GH-26379 is a backport of this pull request to the 3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 26, 2021
… as context var (pythonGH-21199)

(cherry picked from commit 46db39d)

Co-authored-by: Peter Law <[email protected]>
@bedevere-bot
Copy link

GH-26380 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label May 26, 2021
iritkatriel pushed a commit that referenced this pull request May 26, 2021
… as context var (GH-21199) (GH-26379)

(cherry picked from commit 46db39d)

Co-authored-by: Peter Law <[email protected]>
iritkatriel pushed a commit that referenced this pull request May 26, 2021
… as context var (GH-21199) (GH-26380)

(cherry picked from commit 46db39d)

Co-authored-by: Peter Law <[email protected]>
@PeterJCLaw PeterJCLaw deleted the bpo-41147-document-redirect-stdout-uses-context-variable branch January 16, 2022 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants